• go.mod is excluded by !**/*.mod
• go.sum is excluded by !**/*.sum

• .golangci.yml (5 hunks)
• dtls/client_test.go (1 hunks)
• dtls/server/server.go (2 hunks)
• dtls/server/session.go (2 hunks)
• message/codes/code_string.go (2 hunks)
• message/codes/codes.go (2 hunks)
• message/codes/codes_test.go (1 hunks)
• message/option.go (3 hunks)
• message/options_test.go (2 hunks)
• message/pool/message_test.go (2 hunks)
• message/status/status_test.go (3 hunks)
• mux/router.go (1 hunks)
• mux/router_test.go (1 hunks)
• net/blockwise/blockwise.go (9 hunks)
• net/blockwise/blockwise_test.go (5 hunks)
• net/client/limitParallelRequests/limitParallelRequests_test.go (2 hunks)
• net/connUDP.go (6 hunks)
• net/connUDP_internal_test.go (3 hunks)
• net/conn_test.go (3 hunks)
• net/dtlslistener.go (1 hunks)
• net/observation/handler.go (4 hunks)
• net/tlslistener_test.go (4 hunks)
• pkg/cache/cache_test.go (2 hunks)
• pkg/sync/map.go (1 hunks)
• server.go (2 hunks)
• tcp/client/conn.go (1 hunks)
• tcp/client/session.go (2 hunks)
• tcp/client_test.go (1 hunks)
• tcp/coder/bench_test.go (1 hunks)
• tcp/coder/coder_test.go (1 hunks)
• tcp/server/server.go (2 hunks)
• udp/client/conn.go (2 hunks)
• udp/client/conn_test.go (2 hunks)
• udp/client_test.go (10 hunks)
• udp/coder/bench_test.go (1 hunks)
• udp/coder/coder_test.go (1 hunks)
• udp/server/discover.go (3 hunks)
• udp/server/server.go (4 hunks)
• udp/server/session.go (2 hunks)
• udp/server_test.go (1 hunks)

message/status/status_test.go (2)

27-27: The replacement of errors.Is with require.ErrorIs in test assertions is a good practice. It ensures that the test fails immediately if the condition is not met, making the tests more robust and easier to debug.

---

36-36: Using require.ErrorIs for error comparison is consistent with the improvements made in error handling across the project. This change enhances the clarity and reliability of the test cases.

tcp/coder/bench_test.go (1)

15-15: Explicitly declaring enc as an int improves code readability by making the type of enc immediately clear. This is a minor but positive change for maintainability.

udp/coder/bench_test.go (1)

15-15: Just like in the TCP coder benchmark test, explicitly declaring enc as an int here enhances code clarity. Consistency in such improvements across similar files is commendable.

net/conn_test.go (7)

43-43: Switching from assert.NoError to require.NoError ensures that the test halts immediately if an error occurs, which is a best practice for critical error checks in tests.

---

46-46: Consistently using require.NoError for error handling in test teardown logic is a good practice, ensuring that any errors during cleanup are immediately addressed.

---

61-61: Applying require.NoError within the test's goroutine for closing connections enhances the test's reliability by ensuring immediate failure on errors.

---

68-68: Using require.NoError for error handling after dialing enhances the robustness of the test setup phase.

---

72-72: Ensuring no error occurs when closing the connection with require.NoError is crucial for clean test teardown. Good use of require for immediate failure on error.

---

80-80: Switching to require.Error for expected errors aligns with the goal of making tests fail fast on critical conditions. This change improves the test's clarity and reliability.

---

83-83: Consistent use of require.NoError for asserting the absence of errors throughout the test cases is a good practice, ensuring that tests fail fast upon encountering an error.

message/codes/code_string.go (1)

57-57: Replacing fmt.Errorf with errors.New for generating errors where no formatting is needed is a good practice. It simplifies the error creation process and slightly improves performance.

message/codes/codes_test.go (1)

55-55: Replacing require.True with require.Greater for asserting the length of a string provides a more precise and descriptive assertion. This change enhances the clarity and robustness of the test.

dtls/server/session.go (2)

22-22: Changing the ctx field type in the Session struct from atomic.Value to atomic.Pointer[context.Context] is a significant improvement. It leverages the type safety provided by atomic.Pointer, making the code more robust and type-safe.

---

93-93: The updated Context() method to return *s.ctx.Load() directly, thanks to the type safety of atomic.Pointer[context.Context], simplifies the code and avoids the need for type assertion. This is a cleaner and safer approach.

udp/server/discover.go (5)

5-5: Importing the errors package is appropriate for using errors.New for error creation.

---

13-13: Renaming the errors package to pkgErrors to avoid conflict with the standard library's errors package is a good practice when both are needed.

---

47-47: Using errors.New for creating simple error messages without formatting is a good practice for consistency and performance.

---

51-51: Replacing fmt.Errorf with errors.New when no formatting is needed is a good improvement for error creation.

---

67-67: Correctly using a package-specific error (pkgErrors.ErrKeyAlreadyExists) ensures that errors are more descriptive and can be handled specifically.

udp/server/session.go (2)

21-21: Changing the ctx field type from atomic.Value to atomic.Pointer[context.Context] is a more type-safe approach for storing pointers to context.Context in an atomic way.

---

104-104: Using *s.ctx.Load() directly without type assertion is now possible due to the specific type atomic.Pointer[context.Context], enhancing code readability and safety.

message/codes/codes.go (1)

120-120: Replacing fmt.Errorf with errors.New for a simple error message without formatting is a good practice for consistency and performance.

pkg/cache/cache_test.go (2)

121-121: Using require.Equal directly to compare the expected number of found elements with the actual count is a clear and concise way to assert test conditions.

---

148-148: Asserting that no elements are found after a LoadAndDeleteAll operation using require.Equal to check the count is a straightforward way to validate the cache's behavior.

net/dtlslistener.go (1)

73-73: Replacing fmt.Errorf with errors.New for a simple error message without formatting is a good practice for consistency and performance.

server.go (2)

6-6: Importing the errors package is appropriate for using errors.New for error creation.

---

103-103: Using errors.New for creating simple error messages without formatting is a good practice for consistency and performance.

mux/router_test.go (1)

4-4: Replacing the experimental golang.org/x/exp/maps package with the stable maps package is a good practice for ensuring long-term support and stability.

tcp/server/server.go (2)

85-85: Using errors.New for creating simple error messages without formatting is a good practice for consistency and performance.

---

138-138: Replacing fmt.Errorf with errors.New for a simple error message without formatting is a good practice for consistency and performance.

dtls/server/server.go (2)

92-92: The change from fmt.Errorf to errors.New for generating the error message in checkAndSetListener method aligns with the PR's objective to standardize error handling.

---

130-130: The change from fmt.Errorf to errors.New for generating the error message in the Serve method is consistent with the PR's goal to standardize error handling.

net/observation/handler.go (2)

5-5: The addition of the errors import aligns with the PR's objective to standardize error handling.

---

14-14: Renaming the errors package import to pkgErrors to avoid naming conflicts with the standard errors package is a sensible change.

tcp/client/session.go (1)

33-33: The update of the ctx field in the Session struct to use atomic.Pointer[context.Context] improves type safety and code clarity, especially in the Context() method where it simplifies context handling by removing the need for type assertion.

Also applies to: 141-141

net/client/limitParallelRequests/limitParallelRequests_test.go (2)

30-30: The change from fmt.Errorf to errors.New in the do method of the mockClient struct aligns with the PR's objective to standardize error handling.

---

35-35: The change from fmt.Errorf to errors.New in the doObserve method of the mockClient struct is consistent with the PR's goal to standardize error handling.

udp/coder/coder_test.go (1)

81-81: The change from an implicit to an explicit variable declaration (var enc int) is a minor stylistic adjustment that improves the clarity of the variable's type. This is generally a good practice for readability, especially in complex functions. However, ensure that such changes align with the project's coding standards and consider if a rationale should be provided for stylistic changes, especially if part of a larger effort to standardize code style.

message/pool/message_test.go (2)

358-358: Changing the assertion order to require.Equal(t, "seek error", err.Error()) aligns with best practices by placing the expected value first. This improves the readability of test failures and is a good practice to follow in test assertions.

---

389-389: The change to use require.Len(t, data, n) is an improvement, as it leverages a more specific assertion method designed for checking the length of collections. This approach enhances the clarity of test failures by providing more informative error messages.

udp/server/server.go (1)

121-121: Using errors.New for creating simple error messages without formatting is a good practice for consistency and clarity in error handling.

message/options_test.go (2)

267-267: Replacing require.Equal(t, nil, err) with require.NoError(t, err) in test assertions is a good practice for clarity and idiomatic Go code.

---

291-291: The use of require.NoError(t, err) for error assertions in tests is idiomatic and improves the readability of the test code.

net/connUDP_internal_test.go (3)

93-95: The replacement of assert with require for error handling in tests is a good practice, ensuring immediate failure upon encountering an error. This change enhances the robustness of the test suite.

---

211-211: Consider using require instead of assert for payload comparison to maintain consistency and ensure that the test fails immediately if the payloads do not match.

---

227-230: The replacement of assert with require for error handling in tests is consistent with best practices, ensuring immediate failure upon encountering an error. This enhances the robustness of the test suite.

tcp/client/conn.go (1)

169-169: Using errors.New for static error messages, as done here, is a good practice for consistency and clarity in error handling.

message/option.go (2)

115-115: Replacing fmt.Errorf with errors.New for static error messages, as done here, aligns with Go's best practices for error handling.

---

236-236: Using errors.New for static error messages, as done here, is consistent with Go's best practices for error handling.

udp/server_test.go (2)

200-200: The change from a length check to require.NotEmpty for the got variable in the TestServerDiscover function is a positive improvement. It makes the intention of the test clearer and leverages the require package's functionality for more expressive tests.

---

197-203: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-250]

Overall, the test suite in udp/server_test.go is well-structured and follows good practices in Go testing, including the use of require for assertions and proper resource cleanup. The change to use require.NotEmpty enhances the clarity of the test's intention. Keep up the good work!

net/connUDP.go (4)

248-248: The use of errors.New for creating error messages without formatting is a good practice for consistency and performance.

---

398-398: Using errors.New for simple error messages is appropriate and aligns with Go best practices.

---

472-472: The transition to errors.New for creating error messages is correctly applied here, enhancing error handling consistency.

---

611-611: Correctly using errors.New for error creation without formatting parameters. This change improves the readability and performance of error handling.

udp/client_test.go (11)

108-108: Replacing assert.True with require.Equal for checking message types enhances test clarity and failure handling.

---

116-116: Using assert.Equal for message type comparison is more direct and readable than the previous approach.

---

124-124: Correctly using assert.NotEqual for message type assertion improves the specificity of the test.

---

134-134: Switching to require.Equal for message type checks in tests ensures immediate failure on mismatch, which is a good practice.

---

210-210: The use of assert.Equal for message type assertion in a separate message test case is appropriate and improves test accuracy.

---

342-342: Using assert.Equal for message type checks in test cases is a good practice for ensuring the expected behavior.

---

356-356: Correctly using assert.Equal for message type assertion in test cases improves the clarity and accuracy of the tests.

---

472-472: The use of assert.Equal for asserting message types in test cases is appropriate and enhances test clarity.

---

486-486: Applying assert.Equal for message type checks in test cases ensures that the tests are specific and clear.

---

585-585: Using assert.Equal for message type assertion in delete operation tests is a good practice for ensuring the expected behavior.

---

593-593: Correctly using assert.Equal for message type checks in test cases improves the clarity and accuracy of the tests.

tcp/client_test.go (1)

840-840: The change from require.True to require.ErrorIs for the error assertion enhances the test's specificity and robustness by ensuring the exact error is checked. This aligns well with the PR's objective of improving error handling.

net/blockwise/blockwise_test.go (3)

305-305: Changing from assert.Error to require.Error for error assertions ensures that tests fail immediately upon encountering an error, aligning with the PR's objective to enhance testing methodologies.

---

309-309: Changing from require.Len to require.NotEmpty for cache length checks focuses the assertion on the essential aspect of non-emptiness, which is more relevant to the test's objectives and enhances flexibility.

---

311-311: Consistently changing from require.Len to require.NotEmpty for cache length checks across different parts of the file enhances the focus on the essential aspect of non-emptiness, aligning with the PR's objectives.

dtls/client_test.go (1)

211-211: The error handling in the TestConnGetSeparateMessage function has been changed from assert to require, ensuring that the test fails immediately if an unexpected message is received. This change improves the robustness of the test by halting execution on failure, which is especially important in asynchronous or network-dependent tests where continued execution after a failure could lead to misleading results or further errors.

udp/client/conn_test.go (2)

377-377: The error handling in the TestConnGetSeparateMessage function has been changed from assert to require, ensuring that the test fails immediately if an unexpected message is received. This change improves the robustness of the test by halting execution on failure, which is especially important in asynchronous or network-dependent tests where continued execution after a failure could lead to misleading results or further errors.

---

1006-1006: The change from require.True to require.ErrorIs for error comparison in the TestConnRequestMonitorDropRequest function is a significant improvement. It allows for more precise error handling by verifying that the error is exactly what is expected, rather than just checking for the presence of an error. This makes the test more accurate and reliable.

net/blockwise/blockwise.go (9)

207-207: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.

---

210-210: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.

---

219-219: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.

---

586-586: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.

---

597-597: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.

---

761-761: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.

---

719-719: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.

---

666-666: Consider handling the error from payloadFile.Truncate(payloadSize) immediately after its occurrence to ensure that any issues with truncating the file are caught and handled appropriately.

---

207-210: Overall, the replacement of fmt.Errorf with errors.New across various error handling scenarios in net/blockwise/blockwise.go is a positive change that standardizes error creation using the errors package. This aligns well with the PR's objectives of enhancing error handling consistency. No further issues were identified in the segments provided. The rest of the file should be reviewed with the same level of detail to ensure consistency and quality throughout.

Also applies to: 219-219, 586-586, 597-597, 761-761, 719-719

udp/client/conn.go (5)

326-326: The replacement of fmt.Errorf with errors.New for the error message related to token validation is appropriate since the error message does not require formatting. This change aligns with best practices for generating static error messages.

---

945-945: The replacement of fmt.Errorf with errors.New for the error message related to multicast message confirmability is correct, as the error message is static and does not require formatting parameters. This is a good practice for creating more efficient and readable error handling.

---

326-326: Consider adding more specific error messages to enhance debugging and error tracing capabilities. While errors.New("invalid token") is clear, it might be beneficial to include why the token is considered invalid if possible.

---

945-945: For multicast messages, the error message errors.New("multicast messages cannot be confirmable") is clear and informative. However, consider documenting this constraint in the function's comments to improve code readability and maintainability.

---

323-329: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-949]

Overall, the file demonstrates good adherence to Go best practices, including proper error handling, use of atomic operations for concurrency safety, and clear separation of concerns. However, consider the following suggestions to further improve the code:

1. Error Handling Consistency: Ensure that all error messages are consistently informative across the entire file. This aids in debugging and maintenance.
2. Documentation: Adding comments to public types and functions would enhance code readability and maintainability, especially for complex logic or when specific behavior is not immediately obvious from the code itself.
3. Performance Considerations: For functions that are called frequently or are critical to the application's performance, evaluate the potential impact of operations within them. For example, the use of locks and atomic operations should be justified by the need for concurrency safety, and their performance impact should be considered.

• go.mod is excluded by !**/*.mod
• go.sum is excluded by !**/*.sum

• .golangci.yml (5 hunks)
• dtls/client_test.go (1 hunks)
• dtls/server/server.go (2 hunks)
• dtls/server/session.go (2 hunks)
• message/codes/code_string.go (2 hunks)
• message/codes/codes.go (2 hunks)
• message/codes/codes_test.go (1 hunks)
• message/option.go (3 hunks)
• message/options_test.go (2 hunks)
• message/pool/message_test.go (2 hunks)
• message/status/status_test.go (3 hunks)
• mux/router.go (1 hunks)
• mux/router_test.go (1 hunks)
• net/blockwise/blockwise.go (9 hunks)
• net/blockwise/blockwise_test.go (7 hunks)
• net/client/limitParallelRequests/limitParallelRequests_test.go (2 hunks)
• net/connUDP.go (6 hunks)
• net/connUDP_internal_test.go (3 hunks)
• net/conn_test.go (3 hunks)
• net/dtlslistener.go (1 hunks)
• net/observation/handler.go (4 hunks)
• net/tlslistener_test.go (4 hunks)
• pkg/cache/cache_test.go (2 hunks)
• pkg/sync/map.go (1 hunks)
• server.go (2 hunks)
• tcp/client/conn.go (1 hunks)
• tcp/client/session.go (2 hunks)
• tcp/client_test.go (1 hunks)
• tcp/coder/bench_test.go (1 hunks)
• tcp/coder/coder_test.go (1 hunks)
• tcp/server/server.go (2 hunks)
• udp/client/conn.go (2 hunks)
• udp/client/conn_test.go (3 hunks)
• udp/client_test.go (11 hunks)
• udp/coder/bench_test.go (1 hunks)
• udp/coder/coder_test.go (1 hunks)
• udp/server/discover.go (3 hunks)
• udp/server/server.go (4 hunks)
• udp/server/session.go (2 hunks)
• udp/server_test.go (1 hunks)

• .golangci.yml
• dtls/client_test.go
• dtls/server/server.go
• dtls/server/session.go
• message/codes/code_string.go
• message/codes/codes.go
• message/codes/codes_test.go
• message/option.go
• message/options_test.go
• message/pool/message_test.go
• message/status/status_test.go
• mux/router.go
• mux/router_test.go
• net/blockwise/blockwise.go
• net/blockwise/blockwise_test.go
• net/client/limitParallelRequests/limitParallelRequests_test.go
• net/connUDP.go
• net/connUDP_internal_test.go
• net/conn_test.go
• net/dtlslistener.go
• net/observation/handler.go
• pkg/cache/cache_test.go
• pkg/sync/map.go
• server.go
• tcp/client/conn.go
• tcp/client/session.go
• tcp/client_test.go
• tcp/coder/bench_test.go
• tcp/coder/coder_test.go
• tcp/server/server.go
• udp/client/conn.go
• udp/client/conn_test.go
• udp/client_test.go
• udp/coder/bench_test.go
• udp/coder/coder_test.go
• udp/server/discover.go
• udp/server/server.go
• udp/server/session.go
• udp/server_test.go

net/tlslistener_test.go (1)

78-78: The replacement of assert with require in test assertions is a positive change, ensuring that tests fail immediately on error, which aligns with the PR's objectives to enhance error handling and testing methodologies. This change improves test reliability by enforcing critical error handling.

Also applies to: 81-81, 89-89, 112-112, 115-115, 123-123, 128-128, 130-130, 164-164, 167-167, 175-175, 213-213, 220-220, 223-223

• go.mod is excluded by !**/*.mod
• go.sum is excluded by !**/*.sum

• .golangci.yml (5 hunks)
• dtls/client_test.go (1 hunks)
• dtls/server/server.go (2 hunks)
• dtls/server/session.go (2 hunks)
• message/codes/code_string.go (2 hunks)
• message/codes/codes.go (2 hunks)
• message/codes/codes_test.go (1 hunks)
• message/option.go (3 hunks)
• message/options_test.go (2 hunks)
• message/pool/message_test.go (2 hunks)
• message/status/status_test.go (3 hunks)
• mux/router.go (1 hunks)
• mux/router_test.go (1 hunks)
• net/blockwise/blockwise.go (9 hunks)
• net/blockwise/blockwise_test.go (7 hunks)
• net/client/limitParallelRequests/limitParallelRequests_test.go (2 hunks)
• net/connUDP.go (6 hunks)
• net/connUDP_internal_test.go (3 hunks)
• net/conn_test.go (3 hunks)
• net/dtlslistener.go (1 hunks)
• net/observation/handler.go (4 hunks)
• net/tlslistener_test.go (4 hunks)
• pkg/cache/cache_test.go (2 hunks)
• pkg/sync/map.go (1 hunks)
• server.go (2 hunks)
• tcp/client/conn.go (1 hunks)
• tcp/client/session.go (2 hunks)
• tcp/client_test.go (1 hunks)
• tcp/coder/bench_test.go (1 hunks)
• tcp/coder/coder_test.go (1 hunks)
• tcp/server/server.go (2 hunks)
• udp/client/conn.go (2 hunks)
• udp/client/conn_test.go (3 hunks)
• udp/client_test.go (11 hunks)
• udp/coder/bench_test.go (1 hunks)
• udp/coder/coder_test.go (1 hunks)
• udp/server/discover.go (3 hunks)
• udp/server/server.go (4 hunks)
• udp/server/session.go (2 hunks)
• udp/server_test.go (1 hunks)

• .golangci.yml
• dtls/client_test.go
• dtls/server/server.go
• dtls/server/session.go
• message/codes/code_string.go
• message/codes/codes.go
• message/codes/codes_test.go
• message/option.go
• message/options_test.go
• message/pool/message_test.go
• message/status/status_test.go
• mux/router.go
• mux/router_test.go
• net/blockwise/blockwise.go
• net/blockwise/blockwise_test.go
• net/client/limitParallelRequests/limitParallelRequests_test.go
• net/connUDP.go
• net/connUDP_internal_test.go
• net/conn_test.go
• net/dtlslistener.go
• net/observation/handler.go
• pkg/cache/cache_test.go
• pkg/sync/map.go
• server.go
• tcp/client/conn.go
• tcp/client/session.go
• tcp/client_test.go
• tcp/coder/bench_test.go
• tcp/coder/coder_test.go
• tcp/server/server.go
• udp/client/conn.go
• udp/client/conn_test.go
• udp/client_test.go
• udp/coder/bench_test.go
• udp/coder/coder_test.go
• udp/server/discover.go
• udp/server/server.go
• udp/server/session.go
• udp/server_test.go

net/tlslistener_test.go (2)

109-133: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [78-130]

The replacement of assert with require for error handling in tests is a good practice to ensure immediate test failure on critical errors.

---

161-178: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [164-223]

The replacement of assert with require in this test function is consistent with the PR's objective to improve error handling in tests.